Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WET-484 Rotating features for Home and News #2424

Merged
merged 2 commits into from
Oct 15, 2024
Merged

WET-484 Rotating features for Home and News #2424

merged 2 commits into from
Oct 15, 2024

Conversation

Ricokola
Copy link
Contributor

No description provided.

duboisp
duboisp requested changes Sep 18, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My review is incomplete. Please do all the cleanup/update and use only supported component/pattern.

templates/home/home-en.html Outdated Show resolved Hide resolved
templates/home/home-en.html Outdated Show resolved Hide resolved
duboisp
duboisp requested changes Sep 18, 2024
View reviewed changes
templates/home/home-en.html Outdated Show resolved Hide resolved
duboisp
duboisp requested changes Sep 19, 2024
View reviewed changes
templates/news/deprecated/news_v2-en.html Outdated Show resolved Hide resolved
templates/news/deprecated/news_v2-en.html Outdated Show resolved Hide resolved
templates/news/deprecated/news_v2-en.html Show resolved Hide resolved
templates/news/deprecated/news_v2-en.html Outdated Show resolved Hide resolved
@Garneauma
Copy link
Contributor

Pre-approved upon successful review.

@Garneauma Garneauma requested review from duboisp and removed request for Garneauma September 23, 2024 17:26
duboisp
duboisp requested changes Sep 26, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a partial review. I will continue tomorrow.

I only quickly reviewed the html of the working example. I didn't tested the working example neither looked at the documentation.

templates/assets/home-feature-1-520x200.jpg Outdated Show resolved Hide resolved
templates/home/home-en.html Show resolved Hide resolved
templates/home/home-en.html Show resolved Hide resolved
templates/home/home-en.html Outdated Show resolved Hide resolved
templates/home/home-fr.html Outdated Show resolved Hide resolved
templates/news/news-en.html Outdated Show resolved Hide resolved
templates/news/news-en.html Outdated Show resolved Hide resolved
@duboisp duboisp self-assigned this Oct 1, 2024
duboisp
duboisp requested changes Oct 3, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here some comment, I am looking at the documentation now.

templates/news/news-en.html Outdated Show resolved Hide resolved
templates/news/news-en.html Outdated Show resolved Hide resolved
templates/news/news-en.html Outdated Show resolved Hide resolved
duboisp
duboisp requested changes Oct 4, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the inline comment

templates/home/api-en.html Outdated Show resolved Hide resolved
templates/home/api-en.html Outdated Show resolved Hide resolved
templates/home/api-en.html Outdated Show resolved Hide resolved
templates/home/api-en.html Outdated Show resolved Hide resolved
templates/home/api-fr.html Outdated Show resolved Hide resolved
templates/home/api-fr.html Outdated Show resolved Hide resolved
duboisp
duboisp requested changes Oct 4, 2024
View reviewed changes
templates/news/news-en.html Outdated Show resolved Hide resolved
duboisp
duboisp requested changes Oct 7, 2024
View reviewed changes
templates/home/api-en.html Show resolved Hide resolved
templates/home/api-fr.html Show resolved Hide resolved
templates/home/api-fr.html Outdated Show resolved Hide resolved
templates/home/api-fr.html Outdated Show resolved Hide resolved
templates/news/news-doc-en.html Outdated Show resolved Hide resolved
templates/news/news-doc-en.html Show resolved Hide resolved
templates/news/news-doc-en.html Outdated Show resolved Hide resolved
@duboisp
Copy link
Member

duboisp commented Oct 7, 2024

Review and tested locally. The working example page looks good.

duboisp
duboisp previously approved these changes Oct 8, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and tested locally.

Look good to merge.

@duboisp
Copy link
Member

duboisp commented Oct 8, 2024

Note: This change is a Patch change

duboisp
duboisp requested changes Oct 9, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my inline comments

templates/news/news-en.html Outdated Show resolved Hide resolved
templates/news/news-en.html Outdated Show resolved Hide resolved
templates/news/news-fr.html Outdated Show resolved Hide resolved
@Garneauma
Copy link
Contributor

@duboisp Thank you for the suggestions. It is much better that way.

duboisp
duboisp approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and tested locally

It looks good.

This is a patch change.

@Garneauma Garneauma merged commit 8a4fcc1 into wet-boew:master Oct 15, 2024
1 check passed
@Garneauma Garneauma added this to the v15.7.0 milestone Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Garneauma Garneauma Garneauma left review comments

@duboisp duboisp duboisp approved these changes

Assignees

@duboisp duboisp

Labels
None yet
Projects
None yet
Milestone
v15.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Ricokola @Garneauma @duboisp